fix(antigravity): comprehensive fix for user skills, bypass permissions, subagent execution - #3890
fix(antigravity): comprehensive fix for user skills, bypass permissions, subagent execution#3890KhoiFishGST wants to merge 5 commits into
Conversation
|
This was a comprehensive fix for agy as I have been trying to get agy harness to work well (like claude code). This does everything that claude code does well for me (subagent calling, skills, and agents listed in tab in the webui). I will provide a video demo soon (tomorrow) and also check issues reported so far and tag them here. @PattaraS for visibility. |
9002366 to
508b5c9
Compare
|
@KhoiFishGST This PR is a Bug fix, Feature, or UI / frontend change but the Demo section is missing or only contains a placeholder. These change types require a screenshot or screen recording so reviewers can see the new behaviour without checking out the branch. Please update the Demo section with:
Use |
KhoiFishGST
changed the title
2026-08-03.07-55-57.mp4Here's a video of it working, agents running and listed under the agents tab. There's one approval message that comes up that gets triggered somehow. The agents run anyway, and once you hit approve it goes away. I can look at this as a follow-up; this diff is getting much too large as it is. Let me know if you guys need anything from me. This makes my agy workflow great; I can work in the browser with antigravity, with subagents! |
508b5c9 to
64e72b7
Compare
Running agy through Omnigent lost most of what agy was doing. This brings the web UI to parity with what the terminal already showed. Every fix below was found by reading live agy RPC traffic and verified against real sessions; the recorded frames are checked in as fixtures. **Plugin skills were missing.** An omnigent-spawned agy gets an isolated `--gemini_dir`, and nothing seeded the user's plugins into it, so `agy plugin list` was empty under Omnigent while identical outside it. The bridge now symlinks `config/plugins` and copies `import_manifest.json`. **The slash menu offered Claude's skills.** The skill-source registry had no antigravity family, so agy sessions fell through to the claude-native provider. agy now has its own five sources, with plugin skills namespaced `<plugin>:<skill>` and enabled only when `plugin.json` is present. **`--dangerously-skip-permissions` was unreachable.** claude-code exposes its bypass in the new-chat dialog; agy had no equivalent, so the flag could only be set by hand-editing launch args. Added as a capability with the same danger banner. **Sub-agents forked duplicate top-level sessions.** agy spawns each sub-agent as its own cascade, and a working sub-agent is always more recently active than the parent idling behind it — so the rotation detector read every spawn as a `/clear` and dragged the pane onto the child. Children are now identified by `trajectoryMetadata` and skipped. **Cold start could bind a stranger's agy.** With several agy processes alive, a session could attach to another one's RPC port and mirror its conversation. Ownership is now confirmed after `StartCascade`. Port attribution also moved from shelling out to `lsof` — an undeclared dependency absent from many images, and unavailable on Windows — to psutil, which is already a dependency, with a `/proc/net/tcp` fallback. **Replies duplicated and truncated.** The streaming reader stamped a constant `"index": 0` on every text delta, and the server discards any chunk whose index does not advance — so the first chunk rendered, the rest were dropped, and the unretired buffer replayed to later subscribers. Deltas now carry a real index, and the live block is closed on both the stream and poll paths. **No tool call was ever mirrored.** agy serves each step at two fidelities: the snapshot RPC carries `metadata.toolCall` and `plannerResponse.toolCalls`, while the live stream strips both (each embeds a `thinkingSignature` blob). The mapper was built against the snapshot, so streamed turns recorded 611 tool outputs against 0 invocations — naked result blobs, most keyed to invented `_orphan_N` ids, with `view_file` and `invoke_subagent` results dropped entirely. Both items now derive from the result step, which both shapes deliver in full, keyed on its own `(trajectory, step)` identity so a stream->poll fallback cannot re-key a pair. **Sub-agent work was invisible.** agy names each sub-agent's cascade, role and type on the parent's `INVOKE_SUBAGENT` step, but nothing mirrored them, so a four-reviewer dispatch showed one opaque tool call and an empty Agents rail. Each child now gets a child session and a mirror loop. `invoke_subagent` is fire-and-forget — its step reaches DONE while the child runs on for minutes — so each mirror ends on its own child's turn closing, with agy's run status as the backstop for a turn that never closes. Test plan: - 730 passed, 1 skipped across the antigravity selection; pre-commit clean. - 6 stream-projection fixtures are verbatim live frames — the shape that had no coverage, which is why the tool-call bug shipped. - Every fix verified end-to-end against a live agy: `agy plugin list` A/B, the `/skills` panel, live SSE captures for the delta index, and a replay of the real conversations for tool calls (18 tool steps -> 18 complete pairs, both RPC shapes agreeing) and sub-agents (children that had recorded 1 item each now mirror their full transcripts and close). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Khoi Nguyen <khoifish@goodstorytime.com>
64e72b7 to
e05021a
Compare
The E2E UI Required gate rejected the PR: the new-chat dialog gained agy's permission control with only Vitest coverage under web/, and no Playwright test exercising it. The gate is right — this is the toggle that arms `--dangerously-skip-permissions`, and the repo requires a UI test for user-facing UI changes. Two tests, driving a real browser against the stubbed landing picker: * arming the bypass raises the red danger banner and rides along to `POST /v1/sessions` as `terminal_launch_args: ["--dangerously-skip-permissions"]`; * leaving it alone sends NO launch args, so a session cannot silently inherit the bypass the user never chose. The banner assertion is the point of the first test as much as the flag is. agy fires no pre-tool hook, so once the bypass is armed Omnigent cannot re-gate individual tools — the warning is the only thing between the user and an agent that edits any file and runs any command without asking. The test also asserts the banner is ABSENT before opting in, so it cannot decay into permanent furniture that users learn to ignore. Both reuse the module's existing `_antigravity_native_agents_body` stub rather than adding a second one. Test plan: - Both pass in a real chromium run (2 passed), and the whole `test_start_session.py` file passes (22 passed). - Each assertion verified to bite: emptying the flag's `args` fails the launch-args assertion, and suppressing the banner fails the visibility assertion. - pre-commit clean. Signed-off-by: Khoi Nguyen <khoifish@goodstorytime.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
I merged tip of main into the branch and resolved requested tests (e2e UI test ran and passed). Everything green. Diff is large but tests are about 62% of the diff. About ~1k runtime lines impacted (~38% of the diff). Tested on my end and things still work as expected. Let me know if you need anything else. Excited to see this thing land! |
|
Hey all! I would love some feedback for this, and would happily make changes, split up the changes etc. if required. Please let me know. |
|
/review |
PattaraS
left a comment
PattaraS
left a comment
There was a problem hiding this comment.
Reviewed the full diff plus the surrounding code the change leans on (inflight_text.py, _detect_rotated_cascade, the codex sibling handler, the harness alias tables) and the six new fixtures.
The two load-bearing fixes hold up, and I checked the wire-shape premise rather than taking the description's word for it: in stream_*_done.json the tool steps carry metadata.toolAction with an empty toolCall block, and stream_planner_tool_call.json has no toolCalls — so keying the pair on the invocation really did lose every streamed tool call, and toolAction really does discriminate (both planner fixtures lack it). The delta-index bug is likewise real: inflight_text.py:608 drops any chunk whose index <= last_index, so a constant 0 silenced everything after the first, the message never saw final, never retired, and got replayed truncated to later subscribers. len(forwarded) is monotonic by construction. Nice work on _close_planner_delta_stream mirroring the mapper's modifiedResponse-over-response precedence — that's what keeps the buffer byte-equal to the committed item.
Five comments inline. One is a real defect (the psutil floor), one is a design question I'd want answered before merge (the skills truth-source), and three are minor.
On your two flagged concerns: the third subagent-start handler is genuinely thinner than codex's — the title is the idempotency key, and _find_subagent_child_by_title already existed to support it — so following the pattern over refactoring two working harnesses looks right to me. Deferring live in-progress tool cards is a reasonable scope line.
Process note, not a blocker: four of the nine fixes touch disjoint files and had no coupling to the rest (psutil/lsof, the skills provider, the permissions toggle, plugin seeding). Those could have shipped as their own PRs and left a much smaller surface for the two hard ones. Not worth re-cutting now — the live-agy verification is the expensive part and a re-split makes you redo it per slice — but worth reaching for next time a change spans this many independent causes.
Reviewed with Claude Code.
| """ | ||
| try: | ||
| conns = psutil.Process(pid).net_connections(kind="tcp") | ||
| except (psutil.Error, OSError): |
There was a problem hiding this comment.
The >=5.9 floor permits a version where this path always fails.
Process.net_connections() was added in psutil 6.0.0 (2024-06-18) as the rename of Process.connections(); pyproject.toml:51 pins psutil>=5.9,<8. On 5.9 the call raises AttributeError, which is caught by neither psutil.Error nor OSError — so it propagates instead of falling back to lsof, making discovery worse than before on that floor rather than better.
The lockfile resolves to 7.2.2, so CI and normal installs are fine; it's --resolution lowest or a downstream consumer with an older pin that breaks. Bump the floor to >=6 (matching the API you actually call) or widen the except to include AttributeError.
|
|
||
| :param ctx: Session discovery context. ``home`` is the real user home: agy | ||
| runs under a bridge-owned ``--gemini_dir`` whose plugins are linked back | ||
| to the real tree, so the real home is the truthful source either way. |
There was a problem hiding this comment.
The menu can list skills agy won't resolve.
This says the real home is truthful "either way," which holds for plugins — _seed_isolated_agy_plugins symlinks config/plugins back to the real tree — but not for the other three sources _agy_skill_dirs enumerates. Global (antigravity-cli/skills), Shared (~/.gemini/skills) and builtins are read from the real home here, yet agy runs under the bridge-owned --gemini_dir (orchestration.py:4343) where nothing seeds them.
So a skill in ~/.gemini/antigravity-cli/skills/ would appear in /skills and then fail to expand when sent to agy as plaintext — which is the exact failure mode pi_host_skills exists to avoid ("listing anything would risk surfacing a command the harness can't run").
test_antigravity_provider_surfaces_all_five_agy_sources asserts the current behaviour, so I take it this is deliberate — what's the reasoning? Either seed the other three alongside plugins, or narrow the provider to what the isolated Gemini dir actually contains.
| # Sub-agent mirrors poll until their parent step settles; a | ||
| # reader teardown mid-flight must not leave them polling a | ||
| # cascade whose agy is going away. | ||
| *state.subagent_mirrors.values(), |
There was a problem hiding this comment.
Grandchild mirrors are unreachable at teardown.
child_state in _mirror_subagent_cascade gets its own subagent_mirrors, and the _process_committed_step → _maybe_mirror_subagents call inside that loop will populate it if a child spawns its own sub-agent. This drain only walks the parent's dict, so a depth-2 mirror holds no reachable reference and keeps polling after the reader is gone.
You already read nestingDepth in _summary_is_child_trajectory — if agy caps nesting at 1 this is unreachable and worth a comment saying so; if it doesn't, the drain needs to recurse into each child's mirrors.
| ) | ||
| await _post_event(client, child_session_id, _status_event(_STATUS_IDLE)) | ||
| return | ||
| quiet_polls = 0 |
There was a problem hiding this comment.
This reset makes the backstop unbounded rather than one-shot.
When the child has been quiet for 60 polls but agy reports it not-idle, zeroing quiet_polls restarts the whole window — so a child agy never marks idle re-checks every 60s for the life of the session.
The direction is right (a wrong "idle" truncates a transcript, which is the worse failure), but _SUBAGENT_QUIESCENT_POLLS' "generous" framing reads as a single grace period rather than an indefinite re-check loop. Worth saying so in the comment, or backing the interval off after the first failed check.
| continue | ||
| if summary.get("trajectoryType") != _TRAJECTORY_TYPE_CASCADE: | ||
| continue # never rotate to a subagent/child trajectory | ||
| continue # never rotate to a non-cascade trajectory |
There was a problem hiding this comment.
Stale comments left behind — including the exact claim this PR disproves.
The module-level comment at :184 still says a subagent/child trajectory "carries a different trajectoryType", which _summary_is_child_trajectory documents as byte-identical to a real root. _detect_rotated_cascade's docstring bullet ("Consider ONLY root cascades … a subagent/child trajectory is never a rotation target") likewise still attributes child-exclusion to the trajectoryType check, without mentioning the new metadata test now doing that work.
Both would lead the next reader to think the type check is load-bearing for child exclusion when this line is really just filtering non-cascade types.
|
|
Thanks for reviewing @PattaraS! I will resolve your findings, and get back to you soon. |
Follow-up to the agy harness work, resolving reviewer feedback on omnigent-ai#3890. - Bump the psutil floor to >=6: the connect-RPC port discovery calls Process.net_connections(), which 5.9 spells connections(). The AttributeError there is neither psutil.Error nor OSError, so it escaped the fallback instead of degrading to lsof. - Seed the Global and Shared agy skill trees into the isolated Gemini dir alongside plugins, so the /skills menu cannot offer a skill agy would fail to expand. The other two sources need nothing: agy recreates its builtins under any --gemini_dir, and the workspace tree is not under it. - Take a sub-agent's own nested mirrors down with it: a child's steps run the same path as the parent's, so a nested INVOKE_SUBAGENT registered a grandchild the reader's teardown drain never walked. - Back the sub-agent quiescence window off after each veto instead of resetting it flat. agy answering "still running" can only veto the close, so a flat window re-asked every minute for the whole session. - Fix two comments still attributing child exclusion to trajectoryType, which a subagent reports byte-identically to a root. - Use a per-step chunk counter for planner delta indices. The forwarded byte offset moves backwards on a shorter post-moderation rewrite, and the server drops any chunk that does not outrank the last accepted one, so the closing final chunk was discarded and the block never closed. - Prefer an exact match before the prefix scan in _arguments_from_body so a suffixed sibling key cannot shadow the argument that was asked for. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Follow-up to the agy harness work, resolving reviewer feedback on omnigent-ai#3890. - Bump the psutil floor to >=6: the connect-RPC port discovery calls Process.net_connections(), which 5.9 spells connections(). The AttributeError there is neither psutil.Error nor OSError, so it escaped the fallback instead of degrading to lsof. - Seed the Global and Shared agy skill trees into the isolated Gemini dir alongside plugins, so the /skills menu cannot offer a skill agy would fail to expand. The other two sources need nothing: agy recreates its builtins under any --gemini_dir, and the workspace tree is not under it. - Take a sub-agent's own nested mirrors down with it: a child's steps run the same path as the parent's, so a nested INVOKE_SUBAGENT registered a grandchild the reader's teardown drain never walked. - Back the sub-agent quiescence window off after each veto instead of resetting it flat. agy answering "still running" can only veto the close, so a flat window re-asked every minute for the whole session. - Fix two comments still attributing child exclusion to trajectoryType, which a subagent reports byte-identically to a root. - Use a per-step chunk counter for planner delta indices. The forwarded byte offset moves backwards on a shorter post-moderation rewrite, and the server drops any chunk that does not outrank the last accepted one, so the closing final chunk was discarded and the block never closed. - Prefer an exact match before the prefix scan in _arguments_from_body so a suffixed sibling key cannot shadow the argument that was asked for. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Khoi Nguyen <khoifish@goodstorytime.com>
Signed-off-by: Khoi Nguyen <khoifish@goodstorytime.com> # Conflicts: # tests/runner/test_app_sessions_native_terminals_runtime.py
The sub-agent start path resolved its symbols through the sessions wildcard imports, which main has since replaced with explicit blocks. The references now fall through to NameError on the first external_antigravity_subagent_start event. Import each symbol from its owning module, matching how the codex equivalents are already listed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Khoi Nguyen <khoifish@goodstorytime.com>
a88f368 to
15a24ef
Compare
|
@PattaraS, I have fixed the issues you've mentioned, and also merged tip of main into the branch and resolved a conflict from the tip of main merge. I re-ran with a fresh build, and everything works from my end (I tested subagents with agy, like before). Unfortunately, psutil floor bumps uv.lock, which triggers the security gate failures. As far as I know, I think these happen on main as well, so not related to this PR. Please let me know if you need anything else. Looking forward to seeing this land! |
4 participants
Summary
Running agy through Omnigent lost most of what agy was doing. The terminal showed
the real session; the web UI showed a subset of it. This brings the two to parity.
Everything here was found by reading live agy connect-RPC traffic and verified
against real sessions. The recorded frames are checked in as fixtures — including
six that capture a wire shape the suite had never covered, which is why the
tool-call bug below could ship green.
ELI5. agy reports what it is doing over two different channels. We read the fast
one, which leaves fields out, while the code assumed the complete one. So the web UI
quietly dropped tool calls, sub-agents, and parts of replies.
Nine fixes, each independently reviewable in the diff:
agy plugin listempty under Omnigent, fine outside--gemini_dirnever seeded--dangerously-skip-permissionsunreachable from the UI/clearStartCascadelsofshelled out for port attribution"index": 0toolCall/toolCallsThe last two are the substantial ones.
Tool calls. The stream omits exactly the two fields tool correlation depended on
— each embeds a
thinkingSignatureblob, and the typed body (runCommand,viewFile, …) already describes the call. So invocations were never emitted; knownresult types fell back to a FIFO allocator whose queue was necessarily empty and
minted
_orphan_Nids; and unknown types (view_file,invoke_subagent) failed thetool-result test and were dropped silently. Across 10 recorded conversations: 611
outputs, 0 invocations. Both items now derive from the result step — which both
shapes deliver in full — keyed on its own
(trajectory, step)identity, so astream→poll fallback cannot re-key a pair. Classification moved to
metadata.toolAction, present on exactly the tool steps in both shapes and inyear-old fixtures, so an agy tool type this mapper has never seen still reaches the UI.
Sub-agents. agy runs each as its own cascade and names its id, role and type on
the parent's
INVOKE_SUBAGENTstep. Omnigent already read that metadata — only toignore those cascades, since a working sub-agent is always more recently active
than the parent idling behind it and otherwise looks like a
/clear. Each child nowgets a child session and a mirror loop reusing the same mapper. One subtlety:
invoke_subagentis fire-and-forget — its step reaches DONE while the child runs onfor minutes (measured: a child worked 23s past its spawning step's
completedAtandran 35 steps) — so each mirror ends on its own child's turn closing, with agy's
run status as the backstop for a turn that never closes cleanly.
Server-side this adds a third
*_subagent_startevent alongside claude's and codex's.It needs less machinery than either: agy's cascade id is already stable and unique, so
the title
"<role>:<cascade id>"is both the idempotency key and correctly parsed bythe Agents rail's existing first-colon split — no display helper, no label-scan lookup.
Test Plan
730 passed, 1 skippedacross the antigravity selection;pre-commitclean.28 files: +3515 / −633 — 1297 production, 1768 test, 274 fixture, 176 web.
Automated:
coverage of that shape at all, which is why every test passed while production
lost data.
view_file, agy's own toolnames, identical call ids from both RPC shapes, the planner staying silent on
both, and a sweep asserting no fixture can mint an orphan id.
payload, a rejected registration starting no mirror, a child's steps landing in the
child session, the mirror outliving the spawning step, a stalled child closed
from agy's run status, and a quiet-but-running child not closed.
cascade_idrejected, and a colon inside a model-authored role.Each regression test was observed failing for the right reason before its fix, and
the load-bearing ones re-checked by reverting the fix (restoring
"index": 0failsthe delta tests; restoring stop-on-parent-settle fails the keeps-polling test).
Manual, against live agy sessions:
agy plugin listA/B inside vs outside Omnigent; the/skillspanel listing agy'sown skills.
pairs, 0 orphans, 0 unpaired, stream and snapshot agreeing on every id and name
(was 8 orphaned, 3 correct, 7 dropped, 0 invocations).
mirror their full transcripts (2 messages + 16 tool pairs, 3 + 43, 2 + 8) and each
closes with a clean
running → idle.Demo
2026-08-03.07-55-57.mp4
Type of change
Test coverage
Coverage notes
The manual verification above needs a live agy process and a real RPC port, so it is
not automated; the unit and integration tests pin each behaviour from recorded
frames. The fixtures are real captures, sanitised only for paths.
Two things reviewers should weigh:
near-identical, and this follows them rather than generalising all three — a
refactor that would touch two working harnesses. Happy to do that instead if
preferred.
deliver PENDING/RUNNING frames, so a live in-progress card is possible; it was left
out to keep this change to the data loss. Easy follow-up.
Changelog
agy sessions now mirror tool calls and sub-agents into the web UI, and no longer
duplicate or truncate replies